[SPARK-32466][SQL][FOLLOW-UP] Normalize Location info in explain plan#29537
[SPARK-32466][SQL][FOLLOW-UP] Normalize Location info in explain plan#29537Ngone51 wants to merge 5 commits intoapache:masterfrom
Conversation
|
cc: @wangyum @maropu @cloud-fan (Note this PR has only updated a few golden files yet for easier review as @maropu mentioned previously) |
|
|
||
| (1) Scan parquet default.store_sales | ||
| Output [4]: [ss_sold_date_sk#1, ss_customer_sk#2, ss_ext_discount_amt#3, ss_ext_list_price#4] | ||
| Output [4]: [ss_sold_date_sk#x, ss_customer_sk#x, ss_ext_discount_amt#x, ss_ext_list_price#x] |
There was a problem hiding this comment.
Sorry for bringing massive changes again.
Do you think we should avoid this change? If it should, I think we could make a custom copy of replaceNotIncludedMsg in PlanStabilitySuite instead of resue.
There was a problem hiding this comment.
After a second thought, this might be useful to understand the query and debug the test failure, let's keep it.
There was a problem hiding this comment.
It's the formatted explained plan. So it doesn't really have Partition Statistics and codegenStageIds.
There was a problem hiding this comment.
It's the formatted explained plan. So it doesn't really have Partition Statistics and codegenStageIds.
Yea, just after I wrote the comment, I noticed that soon... Thanks!
sql/core/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q11.sf100/explain.txt
Show resolved
Hide resolved
Ngone51
force-pushed
the
follow-up-plan-stablity
branch
from
6f46616 to
702bb32
Compare
| val approved = FileUtils.readFileToString(file, StandardCharsets.UTF_8) | ||
| approved == actualSimplifiedPlan | ||
| val expected = FileUtils.readFileToString(file, StandardCharsets.UTF_8) | ||
| expected == actualSimplifiedPlan |
| * plan is not too useful for debugging | ||
| */ | ||
| private def generateApprovedPlanFile(plan: SparkPlan, name: String, explain: String): Unit = { | ||
| private def generateGoldenFile(plan: SparkPlan, name: String, explain: String): Unit = { |
| * Project [c_customer_id] | ||
| */ | ||
| def getSimplifiedPlan(node: SparkPlan, depth: Int): String = { | ||
| def simplifyNode(node: SparkPlan, depth: Int): String = { |
|
Test build #127868 has finished for PR 29537 at commit
|
|
Test build #127875 has finished for PR 29537 at commit
|
|
retest this please. |
|
Test build #127878 has finished for PR 29537 at commit
|
|
Merged to master. |
|
Thank you for your approval and the merging. I created another followup PR to generate all the golden file: #29546 PTAL! |
…e for PlanStabilitySuite ### What changes were proposed in this pull request? This PR regenerates the golden explain file based on the fix: #29537 ### Why are the changes needed? Eliminates the personal related information (e.g., local directories) in the explain plan. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Checked manually. Closes #29546 from Ngone51/follow-up-gen-golden-file. Authored-by: yi.wu <yi.wu@databricks.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org>
5 participants
What changes were proposed in this pull request?
Extract
SQLQueryTestSuite.replaceNotIncludedMsgtoPlanTest.Reuse
replaceNotIncludedMsgto normalize the explain plan that generated inPlanStabilitySuite.Why are the changes needed?
This's a follow-up of #29270.
Eliminates the personal related information (e.g., local directories) in the explain plan.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Updated test.